Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1715 Drop array bound checking #1766

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michaelhkay
Copy link
Contributor

Fix #1715

Drops array bound checking from array:get, arrays-as-functions, and array lookup. Returns () instead of an error FOAY0001 when the index is out of bounds. This brings arrays and maps into closer alignment.

Drops the $fallback argument of array:get()

Adds a new function array:get-if-present() which replicates the old behaviour of array:get().

Functions such as array:put, array:replace, array:insert-before, array:head, array:tail continue to perform bound checking.

@michaelhkay michaelhkay added XPath An issue related to XPath XQuery An issue related to XQuery XQFO An issue related to Functions and Operators Enhancement A change or improvement to an existing feature Reversion PR reverts spec to an earlier status quo Tests Needed Tests need to be written or merged labels Feb 5, 2025
@ChristianGruen
Copy link
Contributor

ChristianGruen commented Feb 5, 2025

Functions such as array:put, array:replace, array:insert-before, array:head, array:tail continue to perform bound checking.

To improve symmetry with sequences, I think we should get rid of the checks for all read function, including array:head and array:tail, to ensure that expressions like $array(1) and array:head($array) can be mutually rewritten (both by users and optimizers). But I agree it’s a good idea to keep the checks for updates.

@michaelhkay michaelhkay force-pushed the 1715-scrap-array-bound-checking branch from a190523 to dc56943 Compare February 11, 2025 17:42
@michaelhkay
Copy link
Contributor Author

In discussion on 2025-02-18, there were considerable reservations about the backwards compatibility impact.

One proposal that may be sufficient to get the proposal through is to add an option in the static context to control this.

Other suggestions were about doing this as an option on individual operations (which we have already tried with array:get) or as a property of the array itself. My own feeling is that such options would be too intrusive.

Marking the PR as blocked awaiting revision.

@michaelhkay michaelhkay added the Blocked PR is blocked (has merge conflicts, doesn't format, etc.) label Feb 19, 2025
@ChristianGruen
Copy link
Contributor

I have read the meeting summary (thanks, @ndw).

I wonder which other cases exist, apart from try/catch clauses, in which backwards compatibility would be an issue? If it’s only try/catch, I am sure we will have other 4.0 changes that are much more consequential (first of all, the disallowance of keywords for names in node constructors).

@michaelhkay michaelhkay force-pushed the 1715-scrap-array-bound-checking branch from dc56943 to fdb8c6d Compare March 10, 2025 12:10
@michaelhkay michaelhkay removed the Blocked PR is blocked (has merge conflicts, doesn't format, etc.) label Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A change or improvement to an existing feature Reversion PR reverts spec to an earlier status quo Tests Needed Tests need to be written or merged XPath An issue related to XPath XQFO An issue related to Functions and Operators XQuery An issue related to XQuery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Array Lookups: partial removal of out-of-bounds checks
2 participants